Fix UMD bundles, making safe to use as required modules#7840
Fix UMD bundles, making safe to use as required modules#7840zpao merged 2 commits intofacebook:masterfrom
Conversation
zpao
force-pushed
the
webpack-alias-dist
branch
from
5ae3cd2 to
ff9fd60
Compare
|
The other case we have is ReactWithAddons, which has a shim that uses a global ReactDOM. However we've never supported using ReactWithAddons as a drop-in replacement in a require system. It should continue to work fine when using globals (eg script tags) just not replacing |
zpao
force-pushed
the
webpack-alias-dist
branch
from
ff9fd60 to
cbe6e61
Compare
|
Ok, got this working and tested for ReactDOM/Server. I haven't tested the AMD stuff (meh) but I think it's right now. @gaearon gave me the thumbs up so I'm going to just merge and get 15.4 RC out. |
| } | ||
|
|
||
| // What is happening here??? | ||
| // I'm glad you asked. It became really to make our bundle splitting work. |
There was a problem hiding this comment.
Too late! I pressed merge.
It became really "hard". Or maybe I wrote "annoying"? I'm going to leave the mystery in there for now :)
|
|
||
| // RequireJS | ||
| } else if (typeof define === "function" && define.amd) { | ||
| require(['react'], f); |
There was a problem hiding this comment.
I think this should be define, not require.
There was a problem hiding this comment.
I think what I have is right. define is used to define the exports, which isn't what we get back from calling f. require just makes sure the module is available and runs code without exports. Same reason we don't do module.exports = f(require('react') for the CommonJS case. It would be define in a UMD wrapper and that is what browserify will build and that we call inside f.
There was a problem hiding this comment.
Looked in the docs and you're right. 😅
(cherry picked from commit 92c84a6)
4 participants
I think this fixes the problem. There's a giant comment in there explaining why and how.
I want to stress that this is a short-term solution. It works and the reasoning is sound but I'm not happy that we need this. It's probably time to start using webpack to bundle but that would have taken me much longer to go do.
Byte-wise this should be fine. We're just adding the code that you can see in there. Since we do this after the bundle process is done, that
require('react')isn't analyzed.Todo:
cc @facebook/react-core
Fixes #7482
Test Plan:
reactandreact-dom.